Skip to content
This repository was archived by the owner on Jul 1, 2021. It is now read-only.

Add bindings for BLS from Chia network #708

Merged
mhchia merged 5 commits intoethereum:masterfrom
mhchia:feature/add-bls-bindings
Jun 24, 2019
Merged

Add bindings for BLS from Chia network #708
mhchia merged 5 commits intoethereum:masterfrom
mhchia:feature/add-bls-bindings

Conversation

@mhchia
Copy link
Member

@mhchia mhchia commented Jun 8, 2019

What was wrong?

ethereum/py_ecc#71

How was it fixed?

Moved from ethereum/py_ecc#72.

Cute Animal Picture

put a cute animal picture link inside the parentheses

@mhchia mhchia mentioned this pull request Jun 8, 2019
Closed
@mhchia
Copy link
Member Author

mhchia commented Jun 8, 2019

CI failed in

@mhchia mhchia force-pushed the feature/add-bls-bindings branch from 48b55d5 to 47d34d6 Compare June 8, 2019 22:48
@mhchia
Copy link
Member Author

mhchia commented Jun 8, 2019

Another unknown CI failure: https://circleci.com/gh/mhchia/trinity/3002?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@carver
Copy link
Contributor

carver commented Jun 10, 2019

Sorry, flaky CI is terrible to work with :(

Working on it:

Yeah, there was a bad eth-keys release. This PR will need to be merged and released and then the test will be resolved: ethereum/eth-keys#62

Another unknown CI failure: https://circleci.com/gh/mhchia/trinity/3002?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Ugh, I haven't seen this one trigger before, but it isn't surprising, since it's sensitive to the VMs speed. We just need to increase the tolerance, I guess. Maybe 10% instead of 5%.

@mhchia
Copy link
Member Author

mhchia commented Jun 11, 2019

@carver No problem! Thanks a lot for the answer 🙏

@carver
Copy link
Contributor

carver commented Jun 11, 2019

eth-keys v0.2.4 is released, so I expect py36-rpc-blockchain to be fixed now 👍

@mhchia mhchia force-pushed the feature/add-bls-bindings branch from 7414e36 to 8538dc7 Compare June 19, 2019 21:27
ChihChengLiang
ChihChengLiang approved these changes Jun 22, 2019
View reviewed changes
Copy link
Contributor

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Generally looks good. I added some style comments.

eth2/_utils/bls_bindings/chia_network/api.py


def aggregate_signatures(signatures: Sequence[BLSSignature]) -> BLSSignature:
signatures_chia = [
Copy link
Contributor

@ChihChengLiang ChihChengLiang Jun 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do tuple here? Looks like the test is happy with tuple.

@ChihChengLiang ChihChengLiang mentioned this pull request Jun 22, 2019
Closed
@mhchia mhchia force-pushed the feature/add-bls-bindings branch from 8538dc7 to 4f8acee Compare June 22, 2019 18:07
@mhchia mhchia merged commit e573ef3 into ethereum:master Jun 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ChihChengLiang ChihChengLiang ChihChengLiang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mhchia @carver @ChihChengLiang